Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Enhancing guides by fixing and making grammar more consistent #33152

Closed
wants to merge 5 commits into from

Conversation

ChrisAHolland
Copy link
Contributor

This pull request is the beginning of me overviewing the entirety of the docs/guides/ folder.

I am aiming to enhance the grammar and make notations more consistent to improve readability and professionalism.

If this pull request is well received and lands, I will continue with the rest of the guides!

Checklist

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 30, 2020
@ChrisAHolland
Copy link
Contributor Author

ChrisAHolland commented Apr 30, 2020

CC @addaleax

@Trott
Copy link
Member

Trott commented May 1, 2020

@nodejs/documentation

@Trott
Copy link
Member

Trott commented May 1, 2020

These changes look good to me, but I suspect it would be better to do this by changes rather than one doc at a time. Would you be open to this approach instead?:

  • Pick one of the types of changes. Maybe using pull request instead of PR or Pull Request. Or maybe capitalizing bulleted entries. But whatever it is, just one thing.
  • Add it to doc/guides/doc-style-guide.md if it makes sense to do so. Put that change in one commit.
  • Make the necessary change across all docs. Basically, every file linted by make lint-md. There are currently 117 of them, and you can find them all with find doc src lib benchmark test tools/doc tools/icu *.md -type f ! -path '*node_modules*' ! -path 'test/fixtures/*' -name '*.md'. Put the updates to these files in a second commit. If the changes are extensive, maybe make it one commit per file.
  • Open a pull request with the two-or-more commits to make the change happen.
  • Bonus points for determining if there is a way to enable a remark-lint rule for the change! But that requires a separate process. Still, noting it in the pull request is helpful.

Two advantages to this approach:

  • It allows each change to be discussed separately. If people like the idea of pull request instead of PR but there is a raging debate over whether to use initial capitalization with bullet points or not, that second thing doesn't hold up the first.
  • I think this approach will ease backporting.

@ChrisAHolland
Copy link
Contributor Author

@Trott Sounds like a good idea to me! Is it possible to have this PR merged, then I can begin work with your suggested process?

p.s. Thanks for the find command, that'll be a lifesaver.

@addaleax
Copy link
Member

addaleax commented May 7, 2020

@Trott Sounds like a good idea to me! Is it possible to have this PR merged, then I can begin work with your suggested process?

Yes, as long as you are okay with the potentially increased backporting workload for you that Rich mentioned. 👍

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 7, 2020
@ChrisAHolland
Copy link
Contributor Author

@Trott Sounds like a good idea to me! Is it possible to have this PR merged, then I can begin work with your suggested process?

Yes, as long as you are okay with the potentially increased backporting workload for you that Rich mentioned. 👍

I'm okay with that, I don't think I will run into any issue. Thanks for the help!

@mhdawson
Copy link
Member

mhdawson commented May 8, 2020

@ChrisAHolland what would also be useful is a list of the changes you will be making across the docs. For example changing to use pull request instead of PR. Documenting those things somewhere in a guide would be good to make sure we stay consistent after your pass. We might even be able to get the linter to enforce some of them.

@Trott
Copy link
Member

Trott commented May 8, 2020

Documenting those things somewhere in a guide

"somewhere in a guide"?

Happy to report that we already have exactly such a guide! https://github.com/nodejs/node/blob/master/doc/guides/doc-style-guide.md

@ChrisAHolland
Copy link
Contributor Author

@mhdawson @Trott That sounds like a good idea to me. Should I add the rule to the guide as I create pull requests for each type of edit I make?

@Trott
Copy link
Member

Trott commented May 9, 2020

Should I add the rule to the guide as I create pull requests for each type of edit I make?

In my opinion, yes, that would be ideal.

@ChrisAHolland
Copy link
Contributor Author

Should I add the rule to the guide as I create pull requests for each type of edit I make?

In my opinion, yes, that would be ideal.

Sounds good!

@addaleax
Copy link
Member

Landed in 441e703

@addaleax addaleax closed this May 15, 2020
addaleax pushed a commit that referenced this pull request May 15, 2020
PR-URL: #33152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
codebytere pushed a commit that referenced this pull request May 16, 2020
PR-URL: #33152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@codebytere codebytere mentioned this pull request May 18, 2020
codebytere pushed a commit that referenced this pull request Jun 7, 2020
PR-URL: #33152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
codebytere pushed a commit to codebytere/node that referenced this pull request Jun 9, 2020
PR-URL: nodejs#33152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@codebytere codebytere mentioned this pull request Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants